Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(SXMC-198): Fix goflow compiler error #44

Merged
merged 4 commits into from
May 10, 2024
Merged

fix(SXMC-198): Fix goflow compiler error #44

merged 4 commits into from
May 10, 2024

Conversation

alk-ktouchie
Copy link
Contributor

@alk-ktouchie alk-ktouchie commented May 8, 2024

Problem

SXMC-198

A go upgrade from 1.17 -> 1.19 introduced a bug fix in go 1.18 which raises an error if any variables are declared but unused. The upgrade itself failed to compile on the CI due to an old variable that was declared but not used, but was merged anyway.

As a result, the pipeline on the PR to change code ownership of the repo failed. We need to unblock this pipeline in order to complete the code ownership transfer before the end of the transition period.

Solution

Taking other graphs as an example, I used the PrinterCtx node to use the declared variable and generated the graph.

Caveats/Notes

  • The generator command in the README didn't seem to work, but I managed to generate the graph using gotestsum, a command I found in the go ci workflow.
  • I had to replace a deprecated package io/ioutil which was caught by the linter.
  • I had to run gofumpt -w to format certain files that were caught by the linter.
  • Also ran into this error in the CI tests: panic: load embedded ruleguard rules: rules/rules.go:13: can't load fmt which @alk-jtessier solved by upgrading the linter.

Prime: @alk-jtessier
cc: @alkemics/sxm-core

@alk-ktouchie alk-ktouchie force-pushed the codeowners branch 8 times, most recently from 83ce5f5 to 5fd8632 Compare May 8, 2024 10:51
@alk-ktouchie alk-ktouchie changed the title test fix(SXMC-198): Fix goflow compiler error May 8, 2024
@alk-ktouchie alk-ktouchie marked this pull request as ready for review May 8, 2024 11:15
@alk-ktouchie alk-ktouchie requested a review from a team as a code owner May 8, 2024 11:15
@alk-ktouchie alk-ktouchie merged commit 53c4174 into master May 10, 2024
2 checks passed
@alk-ktouchie alk-ktouchie deleted the codeowners branch May 10, 2024 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants